Remove --json arg from cloudchamber and containers commands #9815
Conversation
🦋 Changeset detectedLatest commit: 8b87750 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
gpanders
force-pushed
the
push-pvxzxmsqszsk
branch
from
e5974dd to
c969f94
Compare
gpanders
force-pushed
the
push-pvxzxmsqszsk
branch
from
c969f94 to
4c9802d
Compare
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
wrangler
commit: |
gpanders
force-pushed
the
push-pvxzxmsqszsk
branch
4 times, most recently
from
420c555 to
c8a81b9
Compare
gpanders
force-pushed
the
push-pvxzxmsqszsk
branch
from
c8a81b9 to
9ab0f22
Compare
This is a relic from an earlier time. The only command for which this is useful is the "cloudchamber curl" subcommand, in which case it should just be the default and non-configurable anyway.
gpanders
force-pushed
the
push-pvxzxmsqszsk
branch
from
9ab0f22 to
918d683
Compare
There was a problem hiding this comment.
can you unmock the print banner function (vi.unmock("../wrangler-banner");) and add a test to see the banner doesn't print with --json? (historically, the --json flag has been really easy to accidentally break 🥲 )
probably to curl as well.
There was a problem hiding this comment.
Ok done in c87a86e, let me know if I did it wrong.
.changeset/tender-cups-grin.md
Outdated
| "wrangler": patch | ||
| --- | ||
|
|
||
| Remove --json flag from containers and cloudchamber commands |
There was a problem hiding this comment.
can you note that image and curl still have --json
There was a problem hiding this comment.
Updated again to remove reference to curl, since curl doesn't have --json (only images list does).
| json: boolean; | ||
| env?: string; | ||
| imageUpdateRequired?: boolean; | ||
| skipPrompts?: boolean; |
There was a problem hiding this comment.
can this be configured anywhere and if not, is it necessary? looks like its hardcoded to true in one place
There was a problem hiding this comment.
It's necessary to maintain current behavior with how wrangler deploy works, which is always non-interactive.
wrangler cloudchamber apply determines interactivity based on a connected TTY, but wrangler deploy always skips prompts (this is current behavior).
There was a problem hiding this comment.
ah i see now - could you add a comment to skipPrompts to explain this is for deploy?
There was a problem hiding this comment.
Discussed in chat. We'll instead change apply to always be non-interactive, so we don't need skipPrompts after all. 0cf6456
| }; | ||
| } | ||
|
|
||
| export async function loadAccountSpinner({ json }: { json?: boolean }) { |
There was a problem hiding this comment.
why don't we need this anymore?
There was a problem hiding this comment.
The only places it was used were not needed -- after removing those call sites there were none left, so I removed the function completely.
This matches the behavior of "wrangler deploy" which is also always non-interactive.
cloudchamber curl _doesn't_ have a --json flag anymore.
-D is supported as a hidden/deprecated alias
gpanders
force-pushed
the
push-pvxzxmsqszsk
branch
from
72468ea to
8b87750
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
* Remove --json arg from cloudchamber and containers commands This is a relic from an earlier time. The only command for which this is useful is the "cloudchamber curl" subcommand, in which case it should just be the default and non-configurable anyway. * Update changeset entry to include omissions for "curl" and "images list" * Unmock banner to ensure it doesn't print when using --json flag * Make "cloudchamber apply" always non-interactive This matches the behavior of "wrangler deploy" which is also always non-interactive. * Fix lint errors * Remove note from changelog about --json with curl command cloudchamber curl _doesn't_ have a --json flag anymore. * Rename cloudchamber curl -D option to -d to match real curl -D is supported as a hidden/deprecated alias
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
4 participants
Fixes CC-5611.
This is a relic from an earlier time. The only command for which this is useful is the "cloudchamber curl" subcommand, in which case it should just be the default and non-configurable anyway.
It might make sense in the future to have aWe will keep it for--jsonflag for commands likecontainers images list, but if we do add that it should be well-scoped and have proper user requirements, rather than spreading over all of the cloudchamber/containers commands. But for now the output of this command is well-formatted and is easy to parse (same asdocker images).images listtoo. It's trivial to support.